Run find-flaky on CI when new E2E or X% diff by nielsVoogt · Pull Request #8860 · global-121/121-platform

nielsVoogt · 2026-09-17T07:57:26Z

Describe your changes

This adds a job that runs find-flaky (30x) when someone edits (--threshold: 10%), or adds a new E2E test.
Hopefully this will stop flaky tests from entering our codebase

Checklist before requesting a code review

I have performed a self-review of my code
I have addressed all Copilot comments
The changes do not touch the UI/UX
Adding tests is irrelevant for this PR
I have made sure that all automated checks pass before requesting a review
I do not need any deviation from our PR guidelines

Portal preview-deployment

This PR does not have any preview deployments yet.

Copilot

🟡 Changes recommended

Critical workflow and validation-fixture issues prevent reliable flaky-test detection.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds PR-based flaky-test detection for changed or new Portal E2E specs.

Changes:

Adds changed-spec selection tooling and an npm command.
Integrates candidate detection and repeated flaky-test runs into CI.
Adds temporary stable and flaky validation specs.

File summaries

File	Description
`tools/select-changed-e2e-tests.mjs`	Selects qualifying changed E2E specs.
`tools/package.json`	Registers the selector command.
`e2e/portal/tests/TestingCi/PurposlyStable.spec.ts`	Adds stable CI validation coverage.
`e2e/portal/tests/TestingCi/PurposelyFlaky.spec.ts`	Adds temporary flaky-test validation.
`.github/workflows/test_e2e_portal.yml`	Runs candidate detection and flaky-test jobs.

Review details

Suppressed comments (1)

.github/workflows/test_e2e_portal.yml:92

This new job is gated by path-filter, but that filter does not include tools/select-changed-e2e-tests.mjs. After this PR, a future change to the selector alone will make the E2E workflow skip candidate detection, so regressions in this CI-critical script can merge without the workflow exercising them. Include the selector in the path-filter inputs.

    if: ${{ needs.path-filter.outputs.should_skip != 'true' && github.event_name == 'pull_request' }}

Files reviewed: 5/5 changed files
Comments generated: 8
Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot

Copilot review overview

🟡 Changes recommended

The workflow can report false success, and several selector and validation-fixture issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1

Open (1)

Resolution job omits required gating jobs · New

Resolved since last review (8)

This file is intentionally flaky, and the selector always schedules new spec files. While it…
The resolution job does not directly depend on detect-flaky-candidates. If candidate selection…
--retries=0 forces test.info().retry to remain 0, so the added PurposelyFlaky spec fails on…
With --no-renames, deleted paths remain in git diff --numstat. A PR that deletes or renames an…
This workflow_dispatch input is never effective: detect-flaky-candidates is restricted to…
The repository convention in AGENTS.md:156-159 requires a single destructured object parameter,…
This new helper uses naked positional parameters, contrary to the repository requirement that…
The new spec filename contains the spelling Purposly, missing the second e; rename it to…

Previously missed (1)

In code that hasn't changed since last review

Trailing newline skews changed-line percentage

tools/select-changed-e2e-tests.mjs:105

Most source files end with a newline, and split('\n') then produces a trailing empty element. This makes the denominator one line too large, so a file changed by exactly the configured threshold can be incorrectly excluded (for example, 10 changed lines in a 100-line file is calculated as 10/101). Exclude the terminal split element while preserving genuine blank lines.

elwinschmitz

Lets discuss... ;)

Or sit together and do some trial-error-pair-programming on something not-too-complex/big and maintainable/oversee-able...?

elwinschmitz · 2026-10-01T13:13:47Z

+      [
+        path-filter,
+        lint-code,
+        detect-flaky-candidates,
+        test-shard-e2e,
+        find-flaky-changed-tests,
+      ]


Suggested change

[

path-filter,

lint-code,

detect-flaky-candidates,

test-shard-e2e,

find-flaky-changed-tests,

]

- path-filter

- lint-code

- detect-flaky-candidates

- test-shard-e2e

- find-flaky-changed-tests

'Plain' YAML syntax is good enough... the [...]-syntax is just a hack to get it on 1 line...

elwinschmitz · 2026-10-01T13:20:47Z

+import { parseArgs } from 'node:util';
+
+const execFileAsync = ({ command, commandArgs, options = {} }) =>
+  new Promise((resolve, reject) => {
+    execFile(command, commandArgs, options, (error, stdout) => {
+      if (error) {
+        reject(error);
+        return;
+      }
+      resolve(stdout);
+    });
+  });


I think this could be replaces by:

Suggested change

import { parseArgs } from 'node:util';

const execFileAsync = ({ command, commandArgs, options = {} }) =>

new Promise((resolve, reject) => {

execFile(command, commandArgs, options, (error, stdout) => {

if (error) {

reject(error);

return;

}

resolve(stdout);

});

});

import { parseArgs, promisify } from 'node:util';

const execFileAsync = promisify(execFile);

elwinschmitz · 2026-10-01T13:28:08Z

+    base: { type: 'string' },
+    head: { type: 'string', default: 'HEAD' },
+    threshold: { type: 'string', default: '10' },
+    'max-files': { type: 'string', default: '15' },


Suggested change

'max-files': { type: 'string', default: '15' },

base: { type: 'string' },

head: {

type: 'string',

default: 'HEAD'

},

threshold: {

type: 'string',

default: '10'

},

'max-files': {

type: 'string',

default: '15'

},

Would be a bit more readable...

elwinschmitz · 2026-10-01T13:30:46Z

        run: 'npm run lint'

+  detect-flaky-candidates:
+    needs: path-filter


I think this can/should wait on the linting at least to succeed...

elwinschmitz · 2026-10-01T13:31:42Z

+    needs:
+      - path-filter
+      - lint-code
+      - detect-flaky-candidates


Suggested change

needs:

- path-filter

- lint-code

- detect-flaky-candidates

needs: detect-flaky-candidates

This would only need to mention its 'critical path'-dependencies, I'd think... Not "the whole dependency tree"...

elwinschmitz · 2026-10-01T13:37:47Z

+    strategy:
+      fail-fast: false
+      matrix:
+        file: ${{ fromJson(needs.detect-flaky-candidates.outputs.files) }}


This would spawn a runner/job per-changed-spec-file, right?

I wonder how we can 'constrain' this, within some sane limits... 🤔

elwinschmitz · 2026-10-01T13:40:32Z

+
+async function isNewFile({ repositoryRoot, base, path }) {
+  try {
+    await execFileAsync({


Maybe we don't need to make something-sync-async-and-then-await it?
Why not "just run the command sync"?

elwinschmitz · 2026-10-01T13:41:53Z

+  return candidates.slice(0, maxFiles);
+}
+
+function toE2eRelativePath({ path }) {


Weird capitalization...

Maybe just "toRelativePath"?

elwinschmitz · 2026-10-01T13:42:30Z

+  return path.slice('e2e/'.length);
+}
+
+function writeGithubOutput({ candidates }) {


Weird capitalization, it's "GitHub", so: writeGitHubOutput.

elwinschmitz · 2026-10-01T13:45:32Z

@@ -0,0 +1,187 @@
+#!/usr/bin/env node


I think we should be starting to add some linting to this tools-folder...

With adding more and more code, it should all "look alike" and be checked for recommended/best-practice issues etc... (like the MJS-code in the interfaces/portal/scripts-folder..)

And there are probably some more utility-like methods that can be shared around more broadly...

Copilot AI lite review requested due to automatic review settings September 17, 2026 07:57

Copilot started reviewing on behalf of nielsVoogt September 17, 2026 07:57 View session

Copilot AI reviewed Sep 17, 2026

View reviewed changes

nielsVoogt changed the title ~~Niels/investigation.flaky test hunter on pr~~ Run find-flaky on CI when new E2E or X% diff Sep 21, 2026

nielsVoogt requested a balanced review from Copilot September 23, 2026 07:28

Copilot started reviewing on behalf of nielsVoogt September 23, 2026 07:29 View session

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Comment thread .github/workflows/test_e2e_portal.yml Outdated

nielsVoogt added the chore Something that does not affect the end user label Sep 30, 2026

nielsVoogt added 7 commits September 30, 2026 16:06

Flaky test hunter on the CI

a82e2da

Adding purposlyStable as well

0b12fd3

True flakyness

5ed45f5

Addressing copilot comments

9811c1a

Only test E2E's from that PR

7252165

Addressing copilot comments

02be763

Removing test e2e's

70550aa

nielsVoogt force-pushed the niels/investigation.flaky-test-hunter-on-pr branch from cb00552 to 70550aa Compare September 30, 2026 14:06

elwinschmitz reviewed Oct 1, 2026

View reviewed changes

Conversation

nielsVoogt commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your changes

Checklist before requesting a code review

Portal preview-deployment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

elwinschmitz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

nielsVoogt commented Sep 17, 2026 •

edited

Loading